Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use raw-window-handle version 0.6 #132

Merged
merged 6 commits into from
Oct 27, 2023
Merged

Use raw-window-handle version 0.6 #132

merged 6 commits into from
Oct 27, 2023

Conversation

notgull
Copy link
Member

@notgull notgull commented Jul 10, 2023

cc rust-windowing/raw-window-handle#125, rust-windowing/winit#2943

Testing out the new release before it becomes official.

Copy link
Member

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented Web, built on top of forkgull/winit#1 now, until that's merged.

@ids1024
Copy link
Member

ids1024 commented Oct 13, 2023

We should probably add feature flags for raw-window-handle versions, like in rust-windowing/winit#3126.

Edit: Though I guess it's a bit harder to do with adaptive features for arguments. Without some trait magic. If we can support both 0.5 and 0.6 in a clean way, that would be good though.

@ids1024
Copy link
Member

ids1024 commented Oct 13, 2023

Actually, I guess we can't have a function that accepts either something implementing both raw_window_handle_0_5::HasRawWindowHandle and raw_window_handle_0_6::HasRawWindowHandle, because a type could implement both and there's no way to prefer one over the other...

Anyway, if it's going to support only 0.6, it should be released alongside the Winit version that adds support for 0.6.

notgull and others added 2 commits October 21, 2023 08:18
Signed-off-by: John Nunley <[email protected]>
Co-Authored-By: dAxpeDDa <[email protected]>
Signed-off-by: John Nunley <[email protected]>
Signed-off-by: John Nunley <[email protected]>
Signed-off-by: John Nunley <[email protected]>
Signed-off-by: John Nunley <[email protected]>
@ids1024
Copy link
Member

ids1024 commented Oct 23, 2023

CI seems to be failing because the benchmark fails to build. So that also needs to be updated.

Signed-off-by: John Nunley <[email protected]>
pub struct WaylandDisplayImpl {
conn: Connection,
pub struct WaylandDisplayImpl<D: ?Sized> {
conn: Option<Connection>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using an Option, I'd probably just rely on field order, and have a safety comment indicating this requirement.

There's also ManuallyDrop, though the docs for it seem to just recommend using field order for this sort of thing instead of dealing with it: https://doc.rust-lang.org/std/mem/struct.ManuallyDrop.html#manuallydrop-and-drop-order

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather rely on Option than unsafe ManuallyDrop or fragile field ordering.

pub unsafe fn new(display_handle: WaylandDisplayHandle) -> Result<Self, SoftBufferError> {
// SAFETY: Ensured by user
let backend = unsafe { Backend::from_foreign_display(display_handle.display as *mut _) };
impl<D: HasDisplayHandle + ?Sized> WaylandDisplayImpl<D> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why ?Sized? It wouldn't actually be possible to call new with a display: D is D isn't Sized, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way it allows for Context<D> to be coerced into Context<dyn HasDisplayHandle>, and be used like that.

@@ -1,3 +1,5 @@
// TODO: Once winit is updated again, restore this test.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Winit is released, so presumably this can be updated. Though https://github.com/notgull/winit-test will need a release.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@notgull notgull merged commit 0bcd2e2 into master Oct 27, 2023
39 checks passed
@notgull notgull deleted the notgull/rwh-v0.6 branch October 27, 2023 02:15
MarijnS95 added a commit to MarijnS95/softbuffer that referenced this pull request Nov 6, 2023
…Close` again

It seems rust-windowing#132 included a copy-paste typo to trigger draw logic on
`CloseRequested` instead of on `RedrawRequested`.

Signed-off-by: Marijn Suijten <[email protected]>
MarijnS95 added a commit to MarijnS95/softbuffer that referenced this pull request Nov 6, 2023
…Close` again

It seems rust-windowing#132 contains a copy-paste typo to trigger draw logic on
`CloseRequested` instead of on `RedrawRequested`.

Signed-off-by: Marijn Suijten <[email protected]>
notgull pushed a commit that referenced this pull request Nov 6, 2023
…Close` again

It seems #132 contains a copy-paste typo to trigger draw logic on
`CloseRequested` instead of on `RedrawRequested`.

Signed-off-by: Marijn Suijten <[email protected]>
This was referenced Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants